Conversation
* chore: create new folder structure with placeholders * chore: move db connection into database layer * docs: update filetree * chore: minor updates to readme and deduplicate db tests * chore: rename repos to controllers
* chore: set up utils * feat: implement survey model * feat: implement zod validators * feat: implement model agnostic operations * feat: implement survey controllers * chore: temporarily migrate to 'v2' routes * feat: simplify model and add more middleware * feat: add child code generate retries if not unique * chore: streamline zod schema * chore: update test suite to match db constraints * chore: update constants * feat: enforce immutability * chore: update errors * chore: update controllers * feat: ensure children of _SEED_ are not present elsewhere * chore: update zod schema * chore: update tests * chore: update err propagation * docs: add swagger comments * fix: update referral code generator to current constraints * chore: update uniqueness constraint * fix: resolve collision errors * chore: fix comment accuracy * fix: rm unnecessary lines * chore: code review - added REVIEW comments - moved db connection to index.ts - deleted connection folder * chore: folder + middleware refactor for survey * chore: refactor TODO table folders * chore: fix outdated test suite * docs: update filetree diagram * review: Minor refactor after code review * chore: add code uniqueness validation during generation --------- Co-authored-by: Anant Mittal <anmittal@uw.edu>
* fix: async helper functions * chore: referral code -> survey code
* feat: seed mongoose module * feat: seed zod module * feat: seed routes * docs: add swagger comments * feat: location module + migrate location dependencies * chore: update test suites * feat: integrate Location routes * feat: user module * refactor: shared hooks * feat: integrate v2 user routes * chore: flatten user schema, prettify import statements, leave notes for future * chore: minor backwards compatability fixes
feat: Incorporate new RBAC rules
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive refactoring to establish a v2 API architecture with improved permissions, database models, and validation. The changes introduce:
- Migration from
employeeIdtouserObjectIdthroughout the codebase - New v2 routes with permission-based access control using CASL
- Database layer restructuring with Mongoose models and Zod validation for User, Survey, Seed, and Location domains
- TypeScript path alias updates from relative
baseUrlto explicit path mapping - Removal of deprecated v1 utilities and models
Reviewed Changes
Copilot reviewed 58 out of 61 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/tsconfig.json | Updated TypeScript path mapping from baseUrl to explicit paths configuration |
| server/src/utils/authTokenHandler.ts | Refactored to use userObjectId instead of employeeId with typed JWT payload |
| server/src/types/auth.ts | Updated auth types to use userObjectId and added locationObjectId |
| server/src/types/models.ts | Marked for deprecation, updated import paths to new permissions structure |
| server/src/routes/v2/*.ts | New v2 API routes with CASL-based permissions for users, surveys, seeds, locations |
| server/src/routes/v1/*.ts | Legacy routes commented out or updated with new permission imports |
| server/src/permissions/*.ts | New permission system with constants, ability builder, and utility functions |
| server/src/middleware/auth.ts | Enhanced with permission building and latest location derivation |
| server/src/middleware/validate.ts | New Zod-based validation middleware |
| server/src/database//mongoose/.ts | New Mongoose models with hooks for User, Survey, Seed, Location |
| server/src/database//zod/.ts | Zod validation schemas for all database models |
| server/src/database/utils/*.ts | Shared error codes, constants, and validation hooks |
| server/src/index.ts | Updated with v2 routes, Swagger setup, and top-level await for DB connection |
| server/package.json | Added dependencies: zod, swagger-jsdoc, swagger-ui-express |
| client/src/pages/Signup/Signup.tsx | Updated to fetch locations and use new role enum values |
Files not reviewed (1)
- server/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/database/survey/mongoose/__tests__/survey.model.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 59 out of 62 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- server/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!req.authorization) { | ||
| res.sendStatus(403); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The condition !req.authorization will always be false since defineAbilitiesForUser always returns an Ability object (never null or undefined). This check is unnecessary and misleading—remove it or verify that the function can actually return a falsy value.
| hubType: z.enum(HubType), | ||
| locationType: z.enum(LocationType), |
There was a problem hiding this comment.
Using z.enum(HubType) and z.enum(LocationType) is incorrect. These are TypeScript enums, not string literal tuples. Use z.nativeEnum(HubType) and z.nativeEnum(LocationType) instead.
| hubType: z.enum(HubType), | |
| locationType: z.enum(LocationType), | |
| hubType: z.nativeEnum(HubType), | |
| locationType: z.nativeEnum(LocationType), |
| surveyCode: z | ||
| .string() | ||
| .length( | ||
| SURVEY_CODE_LENGTH, | ||
| `Survey code must be exactly ${SURVEY_CODE_LENGTH} characters` | ||
| ), |
There was a problem hiding this comment.
The error message references SURVEY_CODE_LENGTH which is imported as 8, but the test data in survey.zod.test.ts line 15 uses surveyCode: '123456' (6 characters). This indicates either the constant value is wrong or the test data is incorrect, leading to validation failures.
| res.sendFile(path.join(clientBuildPath, 'index.html')); | ||
| }); | ||
|
|
||
| console.log(process.env.NODE_ENV); |
There was a problem hiding this comment.
Debug console.log statement left in production code. Remove this line or replace with proper logging.
| import { subject } from '@casl/ability'; | ||
| import { describe, expect, jest, test } from '@jest/globals'; | ||
|
|
||
| import { AuthenticatedRequest } from '../../types/auth'; |
There was a problem hiding this comment.
Unused import AuthenticatedRequest.
No description provided.